-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Staging to prod #270
Staging to prod #270
Conversation
* feat: contact requests first draft * feat: first working version * feat: make sure user can only send one * feat: fetch mail via rpc * feat: cors * feat: cors via env * fix: typo * feat: Send via smtp directly * feat: check for not more than 3 contact requests in 24h * chore: better variable names * feat: adapt mail template
feat: tests for contact request function (#266) feat: adjust email template
* feat: check for contact request allowed * fix: status code * fix: mail template * feat: reply to
* feat: add monthly rain tables * fix: foreign key * fix: daily weather data * fix: add RLS * feat: created_at column, just to be sure * feat: function for fetching aggregated weather data * chore: formatting * feat: add parameter to limit number of returned months * fix: typo
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This can be merged to prod independently, as it does not require any Frontend changes. It will prepare the frontend to be ready for a) contact request feature and b) Gdk Stats feature. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be nitpicking but I'm missing some basic error handling. Many async request that might fail and are not handled. Also there are some points with unneeded console.logs that seem to be leftover from debugging.
supabase/functions/_shared/checks.ts
Outdated
const { data: senderData, error: senderDataError } = | ||
await supabaseClient.auth.getUser(token); | ||
|
||
console.log(senderData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we lo this?
supabase/functions/_shared/checks.ts
Outdated
.eq("id", senderData.user.id) | ||
.single(); | ||
|
||
console.log(senderLookupData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we log this?
supabase/functions/_shared/checks.ts
Outdated
.not("contact_mail_id", "is", null); // only count sent emails | ||
|
||
if (requestsToRecipientError) { | ||
console.log(requestsToRecipientError); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
console.error?
supabase/functions/_shared/checks.ts
Outdated
.gt("created_at", sub(new Date(), { days: 1 }).toISOString()); | ||
|
||
if (requestsOfLast24hError) { | ||
console.log(requestsOfLast24hError); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
console.error
supabase/functions/_shared/checks.ts
Outdated
const { data: recipientData, error: recipientDataError } = | ||
await supabaseServiceRoleClient | ||
.from("profiles") | ||
.select("*") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we be more sparse which fields we select?
import { checkIfContactRequestIsAllowed } from "../_shared/checks.ts"; | ||
import { corsHeaders } from "../_shared/cors.ts"; | ||
|
||
const SUPABASE_URL = Deno.env.get("SUPABASE_URL"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No check if these vars don't exisit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implemented loadEnvVars
to check for that
const SUPABASE_ANON_KEY = Deno.env.get("SUPABASE_ANON_KEY"); | ||
const SUPABASE_SERVICE_ROLE_KEY = Deno.env.get("SUPABASE_SERVICE_ROLE_KEY"); | ||
|
||
const handler = async (_request: Request): Promise<Response> => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why _request?
const SUPABASE_ANON_KEY = Deno.env.get("SUPABASE_ANON_KEY"); | ||
const SUPABASE_SERVICE_ROLE_KEY = Deno.env.get("SUPABASE_SERVICE_ROLE_KEY"); | ||
|
||
const handler = async (_request: Request): Promise<Response> => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No error handling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implented error handling in later commits
const SMTP_SECURE = Deno.env.get("SMTP_SECURE") === "true"; | ||
|
||
const SUPABASE_URL = Deno.env.get("SUPABASE_URL"); | ||
const SUPABASE_ANON_KEY = Deno.env.get("SUPABASE_ANON_KEY"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No checks for env existence
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implemented loadEnvVars
to check for that
contact_id uuid references auth.users(id) not null, | ||
created_at timestamp with time zone default timezone('utc'::text, now()) not null, | ||
contact_message text, | ||
contact_mail_id text default null -- the resend.io ID of the sent contact mail |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this resend.io thing still relevant?
* feat: first gdk stats * feat: monthly waterings * feat: mostFrequentTreeSpecies * fix: typo * chore: refactoring * fix: error handling and env * chore: comments * feat: more database functions * feat: env var existence check * feat: more distinguishable errors * fix: console.error instead of console.log for errors * chore: cleanup * fix: env variables * fix: more restrictive cors, chore: cleanup
No description provided.